Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

feat: close confirmation modal #1654

Merged
merged 25 commits into from
Feb 24, 2020
Merged

feat: close confirmation modal #1654

merged 25 commits into from
Feb 24, 2020

Conversation

dav1app
Copy link
Contributor

@dav1app dav1app commented Feb 9, 2020

Displays a close confirmation when try closing with backdrop the click or button if props.confirmClose is specified on ModalWindow.

Summary

Check #1651.

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@ghost ghost added Complexity: High More than 256 lines changed. Type: Feature The issue is a request for new functionality. Status: In Progress The issue or pull request is being worked on. labels Feb 9, 2020
@codecov
Copy link

codecov bot commented Feb 9, 2020 •
edited
Loading

Codecov Report

Merging #1654 into develop will not change coverage.
The diff coverage is 81.81%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1654   +/-   ##
========================================
  Coverage    63.86%   63.86%           
========================================
  Files          139      139           
  Lines         3814     3814           
  Branches       797      797           
========================================
  Hits          2436     2436           
  Misses        1152     1152           
  Partials       226      226
Impacted Files Coverage Δ
src/renderer/splashscreen.js 0% <0%> (ø) ⬆️
...ts/Wallet/WalletTransactions/WalletTransactions.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55ebf55...82c576f. Read the comment docs.

@dav1app dav1app marked this pull request as ready for review February 9, 2020 17:06
@ghost ghost added Status: Needs Review The issue or pull request needs a review by a developer of the team. and removed Status: In Progress The issue or pull request is being worked on. labels Feb 9, 2020
@ghost ghost mentioned this pull request Feb 9, 2020
Closed
@alexbarnsley alexbarnsley added Status: Needs Changes The pull request needs additional changes before it can be merged. and removed Status: Needs Review The issue or pull request needs a review by a developer of the team. labels Feb 11, 2020
alexbarnsley
alexbarnsley previously approved these changes Feb 14, 2020
View reviewed changes
@alexbarnsley
Copy link
Member

@davimello28 there seems to be a failing test

@faustbrian
Copy link
Contributor

https://github.com/ArkEcosystem/desktop-wallet/pull/1654/files#diff-6dc9bdfca16f596aee2b06566ee40a74R10 mousedown is no longer used for the closing so the test is outdated and failing.

@dav1app dav1app added Status: Needs Review The issue or pull request needs a review by a developer of the team. and removed Status: Needs Changes The pull request needs additional changes before it can be merged. labels Feb 20, 2020
@alexbarnsley alexbarnsley merged commit 38b3ba4 into develop Feb 24, 2020
@ghost ghost deleted the feat/close-confirmation-modal branch February 24, 2020 23:20
@ghost ghost removed the Status: Needs Review The issue or pull request needs a review by a developer of the team. label Feb 24, 2020
@ghost ghost mentioned this pull request Mar 1, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alexbarnsley alexbarnsley alexbarnsley approved these changes

@faustbrian faustbrian Awaiting requested review from faustbrian

@ItsANameToo ItsANameToo Awaiting requested review from ItsANameToo ItsANameToo is a code owner

@luciorubeens luciorubeens Awaiting requested review from luciorubeens

Assignees
No one assigned
Labels
Complexity: High More than 256 lines changed. Type: Feature The issue is a request for new functionality.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dav1app @alexbarnsley @faustbrian